Avoid error when building file-based libraries #51063
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where building file-based libraries would cause errors. The fix changes how run properties are extracted from project instances, allowing for graceful handling when a project cannot be run (like library projects).
Key changes:
- Modified
RunPropertiesto use a try-pattern method that can fail gracefully - Updated the caching logic to handle cases where run properties cannot be extracted
- Added comprehensive test coverage for building library projects
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Cli/dotnet/Commands/Run/RunProperties.cs |
Refactored to use try-pattern for extracting run properties from projects |
src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs |
Updated to handle null run properties when caching and added debug logging |
test/dotnet.Tests/CommandTests/Run/RunFileTests.cs |
Added test case to verify library projects can be built without errors |
|
@RikkiGibson @MiYanni for reviews, thanks |
jjonescz
added
Servicing-consider
Servicing-approved
and removed
Servicing-consider
labels
|
@RikkiGibson for a review, thanks |
| TargetFrameworkVersion: project.GetPropertyValue("TargetFrameworkVersion")); | ||
|
|
||
| if (string.IsNullOrEmpty(result.Command)) | ||
| return !string.IsNullOrEmpty(result.Command); |
There was a problem hiding this comment.
It seems a little strange for result to contain non-null when the return value is false.
There was a problem hiding this comment.
Yeah, since we already allocated the RunProperties, I returned them, but it might be an anti-pattern, I will fix, thanks.
| return !string.IsNullOrEmpty(result.Command); | ||
| } | ||
|
|
||
| internal static RunProperties FromProject(ProjectInstance project) |
There was a problem hiding this comment.
It looks like a call site of FromProject in src/Cli/dotnet/Commands/Test/MTP/SolutionAndProjectUtility.cs:308 is reimplementing some aspects of FromProject. It doesn't seem like it would lead to incorrect behavior, but, thought I would call it out anyway.
| .Execute() | ||
| .Should().Pass(); | ||
|
|
||
| new DotnetCommand(Log, "run", "winexe.cs") |
There was a problem hiding this comment.
This question isn't really relevant to the fix, but..
What wizardry are you doing to get console output to actually flow out when running a winexe? If I make a new console app from template, change OutputType to WinExe, and dotnet run the project, I don't get any output.
There was a problem hiding this comment.
Output from WinExe is visible when redirected, e.g., dotnet run winexe.cs > out.txt works. And the test utility redirects the output under the hood.
Description and customer impact
Fixes #51064.
This fixes a regression which broke file-based libraries (that's not a mainline scenario for file-based apps, but it's commonly used for example when testing out compiler features or creating minimal bug repros - that's also how I discovered the issue).
Regression
Yes, worked in prior .NET 10 previews.
Risk
Low, this is a small change, with lots of unit testing (pre-existing tests for the mainline
dotnet run app.csbehavior and new tests for the library behavior which regressed).